Skip to content

feat(studio): mirror canvas z-order actions into timeline lanes (track order = default paint order)#2380

Open
ukimsanov wants to merge 19 commits into
mainfrom
feat/track-paint-order-mirror
Open

feat(studio): mirror canvas z-order actions into timeline lanes (track order = default paint order)#2380
ukimsanov wants to merge 19 commits into
mainfrom
feat/track-paint-order-mirror

Conversation

@ukimsanov

Copy link
Copy Markdown
Collaborator

What

Implements the agreed model: track order = default paint order, authored z = advanced override. Canvas z-order menu actions (Bring to Front / Bring Forward / Send Backward / Send to Back) on a timeline clip now ALSO move the clip on the timeline — to the closest track beyond the element it crossed, in the action's direction, that is free over the clip's whole time span; when none is free, a new track is created adjacent to that element (the sanctioned insert renumber). Both writes fold into one undo entry. Clips whose authored z contradicts their lane order get a small "z" badge ("Paint order overridden — see Layers panel"), so overrides are surfaced instead of silently disagreeing with the timeline.

Design (the load-bearing decisions)

  • Render truth stays z. The runtime/renderer is untouched — compositions remain plain HTML where z-index means what CSS says. The studio maintains z↔track consistency at edit time: vertical timeline drags already sync lane→z (shipped in fix(studio): restore golden-branch timeline behaviors dropped by the stack rebuild #2347); this PR adds the missing direction, z-action→lane.
  • Mirror rule: target = closest free lane strictly beyond the crossed neighbor (forward/backward) or beyond the overlap set's extreme (front/back); freeness = the clip's entire [start, end) span; no free lane → insert a new one at the boundary beyond the reference. Same-stacking-context / same-file scoping matches the menu's existing sibling scope, so sub-comp children mirror within their own file's lanes and cross-context cases never arise. Non-clip decorations (no timeline presence) keep the previous z-only behavior. Audio zone untouched.
  • ⚠️ Open product question (M/Bin sign-off wanted): the comparison scope is temporal-overlap — "send to back" goes below the lowest clip it coexists with in time, not below every visual track on the timeline. Less timeline churn; trivially changeable to whole-timeline scope if preferred. Default documented in timelineZMirror.ts's module note.

How

  • timelineZMirror.ts — pure resolver ({kind:"move", displayTrack, persistTrack} | {kind:"insert", insertRow} | null), reusing isLaneFree/timeRangesOverlap and the shared authored-space translation (authoredTrackForLane) — no duplicated machinery. 31 dedicated tests (span-freeness, zone boundary, sparse authored files, sub-comp scoping, determinism).
  • useCanvasZOrderTimelineMirror.ts — runs after the z commit resolves; kind:"move" persists through the same path as a timeline lane drag (optimistic store update, authoredTrack refresh, rollback); kind:"insert" reuses commitTrackInsert's renumber via a shared buildTrackInsertEdits core. The mirror never triggers the lane→z stacking sync, so it cannot fight the z values the action just set (tested).
  • One undo entry: zReorderCoalesceKey is per-gesture-unique (monotonic seq) and both records carry coalesceMs: Infinity — a gesture always folds regardless of persist latency, and distinct gestures can never merge. This also hardens the existing lane-drag move→z fold, which had the same latent >300ms split. The fold test simulates a 400ms gap (failed before, passes after).
  • timelineZOverride.ts + TimelineClip badge — laneIsAbove XOR paintsAbove among temporally-overlapping same-context visual neighbors, using the exact stacking-sync predicates (exported, not copied).

Test plan

  • Unit: 31 resolver tests, mirror wiring (move / insert / null / no-stacking-sync-retrigger / undo fold incl. slow-persist gap / two-gestures-two-entries), badge rule (9), insert-core parity — full studio suite 2161 passing (the 18 telemetry/SnapToolbar failures are the known Node-localStorage environment set, identical on pristine main)
  • All pre-commit gates green (filesize, fallow, lint, format, typecheck)
  • Live pointer-driven verification on a real project: Bring-to-front wrote z AND created a new top track with the correct renumber on disk (no free lane above, both overlapping clips' lanes occupied); Send-to-back correctly stayed z-only (nothing overlaps the clip below in time — the temporal-overlap scope working as designed); the "z" badge renders on clips whose authored z genuinely contradicts lane order; ONE ⌘Z reverts the z write + lane hop + renumber atomically (zero file diff vs the pre-action snapshot)

@ukimsanov

Copy link
Copy Markdown
Collaborator Author

Follow-up commit from live feel-testing:

  • Flashless mirror: the mirrored lane write rode the timing-edit fallback, which full-reloads the preview when there's no GSAP script to soft-swap — but track-only batches change nothing the renderer reads, so they now skip the GSAP round-trip and the reload entirely (single moves AND insert renumbers). Live-verified with an iframe marker surviving the whole gesture.
  • Icons on the four z-order actions (layer-diamond + arrow set; pierced stack for Front/Back). Labels unchanged — they're the standard names across PowerPoint/Illustrator/Figma.
  • New: Close gap / Close all gaps on right-clicking empty lane space (CapCut/Premiere-style, track-scoped, leading gaps included, one undo per action, refuses around locked clips, gap width shown in the menu). Pure math module + 33 tests; persists via the existing atomic batch path. Live-verified: 22.51s gap closed flush on disk, one ⌘Z restored it.

Full studio suite 2196 passing (same 18 environmental failures); all pre-commit gates green.

@ukimsanov

Copy link
Copy Markdown
Collaborator Author

Two more feel-test rounds (954bcf6):

  • Menu order → classical: Bring to Front, Bring Forward, Send Backward, Send to Back (with the layer/arrow icons added in the previous commit).
  • Gap-close blink, properly killed. Journey worth documenting: v1 soft-reloaded by re-running the comp's single inline script — bailed on multi-script comps (most real ones). v2 re-ran ALL inline scripts in document order — passed 2210 unit tests, then live testing on a three.js-heavy composition showed it re-executing scene-init scripts (doubled init warnings) and falling back to a reload anyway. v3 is the correct model: when mutated === false the existing __timelines are still valid and only the runtime's visibility windows are stale, so the sync now runs the soft-reload finalization only (seek + __hfForceTimelineRebind + manual-edits reapply) with zero script execution. Bonus: comps with no GSAP script at all — which previously always remounted — are now flashless too.
  • Live-verified on the three.js composition: Close-all-gaps shifted 4 caption clips with correct cumulative amounts (−2.43s / −9.00s across two gaps), the preview iframe never remounted (marker survived), one ⌘Z reverted everything byte-exact.

Suite 2210 passing (same 18 environmental), all gates green.

@ukimsanov

Copy link
Copy Markdown
Collaborator Author

Final semantic correction from user review (bf09849): forward/backward are a one-element step, bounded. The mirror's lane target is now capped at the next temporally-overlapping element beyond the crossed neighbor — free lane strictly between them if one exists, otherwise a new track inserted immediately beyond the crossed element, never traveling past the second one (the old closest-free-lane search could overshoot, creating a track/paint contradiction the zOverride badge would flag on our own action). Front/back keep whole-set semantics (send-to-back lands at the bottom of the visual zone, above audio). The 3-stacked back-to-back case is pinned end-to-end: resolver → commitZMirrorLaneMove → persisted renumbered tracks strictly between the two neighbors. 82 targeted tests, suite 2218 passing (same 18 environmental), all gates green.

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes on the current head (bf09849). The decomposition and test effort are strong, but these verified correctness and persistence issues can leave Studio state or project files inconsistent:

  1. timelineClipDragCommit.ts:404 — mirror inserts normalize the full timeline and can persist track renumbers into unrelated composition files (and some layouts shift audio). Scope the insert candidate and writes to the selected source-file visual lane family.
  2. timelineZMirror.ts:118sameSourceFile is treated as the stacking-context boundary. A file can contain multiple CSS stacking contexts; use the canonical stackingContextId partition.
  3. useCanvasZOrderTimelineMirror.ts:76 — the crossed-neighbor key omits selectorIndex, so duplicate class selectors resolve to index 0 and can mirror against the wrong clip.
  4. timelineGaps.ts:86 — gap compaction starts at absolute time 0 even for expanded children whose display time is parent-relative. This can move an expanded clip to the wrong display time and persist a different local time.
  5. PreviewOverlays.tsx:254 — an unmatched/no-change z patch resolves successfully and still triggers the lane mirror. Return explicit match/persist status and mirror only after the selected z patch is durable.
  6. timelineTimingSync.ts:450 — multi-clip gap closing performs sequential GSAP mutations. If a later mutation fails, earlier script rewrites remain on disk without the aggregate history fold. Make the mutation atomic or roll back all touched files.
  7. PreviewOverlays.tsx:254 — z persistence and lane mirroring use separate queues. A second rapid gesture can write between the first gesture phases and overwrite file state; serialize the complete z-to-lane transaction.

Please add regression coverage for mixed source files/stacking contexts, duplicate selectors, expanded-lane origins, unmatched patches, late GSAP failure, and overlapping z gestures.

All current CI checks are green; these are behavioral cases not covered by the existing suite.

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial review (independent pass)

I read the current PR description, every existing review/comment, the complete 47-file diff (including deletions), and cross-checked the Git stat at bf09849206d3b944bba4c32a5de514bf6f34bcbe: +3,814 / −577. I am not repeating Miguel's seven findings below; these are two additional failure modes from independently falsifying the expanded-timeline contracts.

Findings

  1. [blocker] “Close gap” can silently move an unrelated clip in another composition file.

    buildChildElements assigns expanded children synthetic display tracks with display.track + result.length (useExpandedTimelineElements.ts:166-177), but useTimelineTrackDerivations groups all expanded and top-level clips solely by that numeric track (useTimelineTrackDerivations.ts:19-26). Those namespaces can collide. The new gap menu then treats that mixed-source array as one lane (useTrackGapMenu.ts:45-63) and persists every resulting shift as one batch.

    Minimal negative proof against this head: expand a host on track 0 with two children; the second child becomes scene.html#c2 on synthetic track 1. Keep an ordinary index.html#foreign clip on authored track 1. The derived lane is:

    track 1: scene.html#c2(start=10,duration=5), index.html#foreign(start=20,duration=5)
    resolveAllTrackGaps => scene.html#c2→0, index.html#foreign→5
    

    So right-clicking what looks like one display lane causes a cross-file batch that rewrites an unrelated root clip. This remains broken even after fixing the already-reported parent-relative-zero issue: the source namespaces are still merged. Partition gap operations by source/expanded owner (or give expanded display rows collision-free identities) and add the two-file regression test.

  2. [important] The new z badge compares leaf z-indices across different documents.

    OverrideCandidate.contextKey is only stackingContextId ?? null (timelineZOverride.ts:35-65), and pairComparable checks only that key plus time overlap (:70-74). The expanded set intentionally contains clips from multiple sourceFiles, and each document commonly uses null for its root stacking context. Leaf z-indices and DOM discovery order are not comparable across those documents.

    A direct proof with two overlapping visual clips—index.html#root on track 0/z 0 and scene.html#child on track 1/z 10, both root contexts—returns both keys from computeZOverrideKeys, even though neither can paint above the other. This produces false override badges in the normal expanded-subcomposition view. Scope the context key by source file, e.g. (sourceFile, stackingContextId), and pin a cross-file test. This is distinct from the existing mirror finding: it affects the badge-only detector even if mirror scoping is corrected.

Strengths

  • The pure gap math, mirror resolver, and soft-reload finalization are well separated and heavily characterized.
  • The move-persist layer has explicit optimistic revision/rollback behavior and the comments make its transactional intent unusually easy to audit.
  • Focused Vitest verification passed: 8 files, 136 tests (timelineZMirror, timelineZOverride, timelineGaps, timelineGapCommit, canvas mirror, timing sync, soft reload, undo restore). CI is also fully complete: 53 success, 1 skipped, 0 failures.

Scope

  • Audited: every changed production hunk and deletion across all 47 files; z-menu → persist → lane mirror; expanded-element identity/track derivation; gap model/commit; lane/z synchronization; timing batch/GSAP fallback; soft reload and undo restore; associated tests and all known callers.
  • Trusting: unchanged server mutation endpoint internals and unchanged runtime rebind implementation; I validated their call contracts from source/tests rather than running a live browser composition. No new auth, traversal, command-injection, or unbounded-resource issue was found in the changed surface.

Verdict: REQUEST CHANGES

Reasoning: The new gap action can batch-persist unrelated clips from different composition files because synthetic expanded tracks collide with root track numbers. The z-override detector also reports cross-document false positives. Both violate the PR's core “lane order mirrors paint order” model; the first is a destructive persistence bug and must be fixed before merge.

— Deepwork

ukimsanov added 19 commits July 14, 2026 03:06
…e z overrides

Track order = default paint order; authored z = advanced override.

- timelineZMirror.ts: pure resolver mapping a successful z-menu action to a
  timeline lane move — closest track in the action's direction that is free
  over the clip's whole span, else a new lane adjacent to the crossed
  neighbor; temporal-overlap scope (default pending product sign-off, see
  module doc); visual zone only; same-file reference scoping; persistTrack
  via the shared authored-space rules. null for non-clips (menu stays
  z-only) and at-extreme/no-overlap cases.
- useCanvasZOrderTimelineMirror.ts: after the z commit resolves, the mirror
  persists the lane move through the same machinery as a timeline lane drag
  (optimistic store update, authoredTrack refresh, rollback); inserts reuse
  commitTrackInsert's renumber via a shared buildTrackInsertEdits core. Both
  writes share one coalesce key (zReorderCoalesceKey) and fold into ONE undo
  entry (test proves it over the real history reducer). The mirror never
  triggers the lane->z stacking sync, so it cannot fight the z values the
  action just set.
- timelineZOverride.ts + TimelineClip badge: clips whose paint order
  contradicts lane order among temporally-overlapping same-context visual
  neighbors (laneIsAbove XOR paintsAbove, the stacking-sync predicates) show
  a 'z' badge — authored z overrides are surfaced instead of silently
  disagreeing with the timeline.
- Timeline.tsx track derivations extracted to useTimelineTrackDerivations
  (600-line cap).
…s slow persists

Live verification caught the z write and the mirrored lane write splitting
into two undo entries: the mirror runs after the z persist's server round
trip, which exceeds editHistory's default 300ms coalesce window under real
latency (the unit test's deterministic clock sat inside it).

zReorderCoalesceKey now mints a per-gesture-unique key (monotonic seq, the
laneChangeGestureSeq precedent) and both records carry coalesceMs Infinity —
distinct gestures can never merge, and one gesture always folds regardless
of write latency. coalesceMs threaded through the persist chain alongside
coalesceKey. Also hardens the existing lane-drag move->z fold, which had the
same latent split. Fold test now simulates a 400ms gap (failed before the
fix, passes after); a two-separate-gestures test asserts two entries.
…ack menu

- Track-only batch moves (the z-mirror's lane hop and the insert renumber)
  skip the GSAP fallback round-trip and the preview reload entirely — the
  renderer never reads data-track-index, and the live DOM patch + optimistic
  store update cover the UI. Mixed batches keep current behavior. Kills the
  canvas blink on mirrored Bring/Send actions (live-verified: an
  iframe-scoped marker survives the whole gesture).
- The four z-order menu items get 16px stroke icons (single layer diamond +
  directional arrow for Forward/Backward; pierced two-layer stack for
  Front/Back); labels unchanged — they are the industry-standard names.
- New track context menu on empty lane space: 'Close gap' (shifts the next
  clip and every clip after it on that lane left by the clicked gap's width;
  leading gaps count, so a single clip with empty space before it compacts
  to 0) and 'Close all gaps' (whole lane contiguous from 0). Pure gap math
  in timelineGaps.ts; persists through the drag path's atomic batch move
  (one undo per action); refuses when a clip that must shift is locked;
  items disable when there is nothing to close.
…ssical z-menu order

Timing edits that rewrote NO GSAP positions (gap closes and moves of
selector-addressed caption clips, zero-delta batches, comps without a
rewritable script) full-reloaded the preview — and the rerun-current-scripts
attempt was wrong for real compositions: re-executing init-style scripts
(three.js scenes, caption engines) is exactly the unsafe case, verified live
by doubled init warnings and a fallback reload anyway.

The correct observation: when mutated === false the existing __timelines are
still valid — only the runtime's clip visibility windows are stale, and the
live DOM timing attributes were already patched. So the no-mutation path now
runs applySoftReloadFinalization only (seek + __hfForceTimelineRebind +
manual-edits reapply), extracted from the soft-reload machinery — zero
script execution. This also un-blinks comps with no GSAP script at all,
which previously always remounted. Rewritten-script soft reloads,
cannot-soft-reload, otherFileChanged, and mutation failures keep their
existing behavior. gsapSoftReload's undo/redo restore section moved verbatim
to gsapUndoRestore.ts for the 600-line cap.

Also: z-order menu items reordered to the classical arrangement (Bring to
Front, Bring Forward, Send Backward, Send to Back).

Live-verified on a three.js-heavy composition: Close-all-gaps shifted 4
caption clips with correct cumulative amounts, the preview iframe was never
remounted (marker survived), and one undo reverted everything.
User-specified semantic: Bring Forward / Send Backward move the clip past
EXACTLY ONE element. The mirror's lane target is now bounded by the next
temporally-overlapping element beyond the crossed neighbor: a free lane
strictly between the two is taken (closest to the neighbor), and when they
are back-to-back a new track is inserted immediately beyond the crossed
element — never past the second one. Previously the resolver took the
closest free lane anywhere beyond the neighbor, which could carry the track
past a second element while the z action only stepped past one — a
track/paint contradiction our own zOverride badge would flag. Front/back
keep whole-set semantics (past everything; back stays above the audio
zone). End-to-end test pins the 3-stacked case through commitZMirrorLaneMove
to the persisted renumbered tracks.
… highlights

- TrackGapContextMenu always renders both rows; an inapplicable action dims
  with a tooltip ("No gap here" / lock reason / "No gaps on this track")
  instead of vanishing into a one-item menu. Width badge only when a gap
  exists under the pointer.
- Hovering an ACTIONABLE row highlights the strip(s) it would close in the
  timeline: the single gap for Close gap, every current gap (leading included)
  for Close all gaps. New resolveAllGapIntervals in timelineGaps.ts reports
  present-state intervals (epsilon-tolerant, overlap-safe), distinct from
  resolveAllTrackGaps' post-compaction starts.
- Click-selecting a single clip paints a quieter tint over its lane's gaps
  (suppressed for marquee multi-selection and during drags; the gap-menu hover
  wins on its own lane). Derivation lives in useTimelineGapHighlights with the
  pure buildTimelineGapStrips exported and unit-tested.
- Strips render in TimelineCanvas with the drop-placeholder geometry (row top
  + clip inset), dashed accent for hover, faint tint for selection.
- Timeline.tsx stayed under the 600-line cap by extracting the scroll-viewport
  plumbing (ResizeObserver width + shortcut-hint sync) into
  useTimelineScrollViewport, behavior unchanged.
One button press / pinch gesture now moves the zoom meaningfully: step
factors 1.25x/0.8x -> 1.5x/(2/3) (kept reciprocal so in+out round-trips) and
pinch sensitivity 0.0035 -> 0.007. Addresses "zooming several times to get
anywhere" feedback; cursor anchoring unchanged.
…panel tracks live z edits

Completes the layers/canvas/timeline sync triangle: the Layers panel was the
one surface whose reorders never reached the timeline, and the one that went
stale when the other two wrote z flashlessly.

- Layers drag -> minimal z + equal-jump lane mirror. handleReorder now uses
  the canvas menu's realization core via resolveZOrderReposition (one
  between-z write when a strict gap exists, band-safe scoped renumber
  otherwise) instead of computeReorderZValues' all-sibling stamp — that
  helper is deleted, completing the #2347 unification follow-up. The drop
  then mirrors into a timeline lane move through the same machinery as the
  canvas menu (new resolveRepositionLaneMove: the clip lands on a free lane
  strictly between its NEW paint neighbors' lanes — nearest clip siblings in
  the desired render order, decorations skipped — else a track insert at
  that boundary; audio zone never crossed). Both writes share one
  per-gesture zReorderCoalesceKey with an unbounded fold window, so a drag
  is exactly ONE undo entry; useCanvasZOrderTimelineMirror's plumbing is
  factored into useMirrorLaneMoveCommit and reused by the new
  useLayerReorderTimelineMirror. A same-slot drop is a hard no-op (new
  order-equality guard in resolveZOrderReposition).
- Panel staleness fix: flashless z commits (skipReload) reload nothing and
  bump no refreshKey, so the panel's z-sorted order went stale while paused.
  handleDomZIndexReorderCommit now bumps a store zEditVersion on apply AND
  rollback; the panel re-collects on it. Verified live: the panel re-sorts
  the instant a drag commits and again on undo.
- Layer click reveal (useLayerRevealOverride): clicking a layer that stays
  hidden at the current frame (animation-parked opacity, non-clip
  display/visibility hides, hidden ancestors) temporarily forces the chain
  visible with live inline styles — exact priors restored on deselect, on
  another reveal, on play, and on unmount; never persisted (file diff == 0
  verified live). Clips keep the existing seek-into-window behavior; the
  override applies on a short defer so a seek-revealed clip needs none.
- layerOrdering's unused hasExplicitZIndex probe (zero callers) removed.

Live-verified on a bed copy: a 2-position layers drag wrote exactly one
element (z 6->23 + data-track-index 15->2), the timeline lane moved without
a reload, and a single Cmd+Z restored the file byte-identically.
…rips

- Click-selecting a clip now lights the WHOLE lane minus its clips — leading
  gap, inter-clip gaps, and the open space after the last clip to the rendered
  end (new resolveLaneEmptyIntervals; displayDuration threaded into the strip
  derivation). Still click-only: any drag/resize suppresses the strips, and a
  marquee multi-select never shows them.
- The gap-menu hover strips drop the dashed border (user feedback) — fill only,
  nudged to 0.18 alpha to keep the same visual weight.
… lift

Clicking a layer in the Layers tab now shows the element as if it were at the
very top of the stack while selected — whatever its authored z or panel
position — extending the reveal override (which already forced hidden chains
visible) with a temporary inline z lift:

- liftElementToTop parks the TRUE effective z in data-hf-reveal-prior-z and
  writes a far-top inline z; a static element gets a layout-preserving
  position:relative with its prior parked in data-hf-reveal-prior-pos. Only
  the RENDERER sees the lift: all three studio z readers
  (readTimelineElementZIndex, getElementZIndex, readEffectiveZIndex) return
  the parked prior while the attribute is present, so the canvas z-menu, the
  zOverride badge, the lane mirror, the stacking sync, and the panel sort
  keep reasoning on the element's real z.
- Strictly ephemeral: exact priors restored on deselect / another reveal /
  play / unmount, each property only while it still holds the value the
  override wrote (a later real edit is never clobbered). File diff == 0
  verified live across a full lift/restore cycle.
- A z-reorder commit CONSUMES an active lift (handleDomZIndexReorderCommit
  reads the parked position for its persist-position:relative static check,
  then drops the attributes) — the committed z becomes the truth and the
  later restore is a guarded no-op.
…ft-restore path

Cmd+Z blinked the canvas on essentially every undo. Three independent causes
in applyUndoRestoreToPreview, each sufficient on its own:

1. Master-view path gate: activeCompPath is NULL at the master view, so the
   'paths[0] === activeCompPath' eligibility check could never match the
   index.html restore and every default-view undo full-reloaded at the first
   gate. Normalized to the codebase-wide 'activeCompPath ?? "index.html"'.
2. Nested identity innerHTML check: the diff compared each identified
   element's innerHTML, but the composition root wraps every clip — any child
   change re-detected at the root rejected the restore. Change detection now
   compares only each element's OWN attribute surface; structure/text
   integrity is still guaranteed by the normalize-residual whole-doc pass
   (text nodes, added/removed elements, and un-identified attrs all remain
   after normalization and force the full reload).
3. id-only identity: elements addressed by data-hf-id / selector (no DOM id)
   fell outside the diff entirely. Identity is now id OR data-hf-id, with the
   live sync resolving either.

Also stop re-running an UNCHANGED GSAP script: attribute-only restores (z,
lane, timing, style — the overwhelmingly common undo) now use the rebind-only
finalization (seek + __hfForceTimelineRebind + manual reapply, zero script
execution — the same path as flashless timing edits), instead of tearing down
and rebuilding live timelines or full-reloading when the script can't be
scoped. A restore whose script text genuinely changed still re-runs it via
applySoftReload, and structural restores (split/delete) still full-reload.

Live-verified on the bed (iframe marker): gap-close undo AND redo both keep
the iframe mounted, live DOM lands on the restored values, disk restored
byte-identically.
…again

TRACKS_LEFT_PAD (48px) — the horizontal sibling of TRACKS_TOP_PAD: empty lane
surface between the sticky gutter and the ruler's 00:00 / the first clips,
scrolling WITH the content.

- The lanes and the ruler realize it as a plain flow spacer between the
  sticky gutter cell and the time-mapped content div, so every
  content-relative computation (clip left = t*pps, beat lines, lane-menu
  time, clip drag deltas) is untouched by construction.
- Canvas-space overlays shift by the pad: playhead (getTimelinePlayheadLeft),
  gap strips, drop placeholder, snap guide, range highlight, marquee clip
  rects, beat SVG; the insert line spans the pad.
- Every pointer->time inverse subtracts it symmetrically: seekFromX, razor,
  range/marquee anchors, asset drops, and the zoom-anchor gutter basis; fit
  pps and the display width account for the consumed viewport width.
- Live-verified: t=0 clip edge, the 00:00 tick, and the playhead line center
  all sit at GUTTER + TRACKS_LEFT_PAD, and a ruler click lands the playhead
  center exactly under the pointer.

Also doubles the timeline zoom sensitivity again (user feedback after
feel-testing the first bump): button steps 1.5x/(2/3) -> 2x/0.5, pinch
0.007 -> 0.014.
The pad before t=0 inherited each row's background and bottom border from the
row wrapper, so it read as track lanes. Lane visuals now live on the cells:
the sticky gutter keeps its own separator (header column stays delineated),
the time-mapped content div carries the row background + separator, and the
pad spacer stays transparent — bare shell background, no lines. The
new-track insertion line also starts at the pad's end instead of crossing it.
The ruler corner's right border drew the header-boundary line through the
ruler strip, so the band didn't read as starting at 00:00. Dropped it — the
boundary line belongs to the track rows below; the ruler stays completely
clean from the panel edge to the first tick, matching the empty left pad.
User decision: the "z" chip on clips never earned its place — dropped
entirely (timelineZOverride.ts + test deleted, TimelineClip badge rendering
and the zOverrideKeys derivation/threading removed). This also eliminates the
review's D2 finding at the root: the badge's cross-document comparison
(stackingContextId ?? null collides across source files in the expanded view)
produced false positives, and there is no longer a detector to mis-fire.
overlapsInTime/paintsAbove lose their export (the badge was their only
external consumer); the paint-order predicate itself is unchanged.
…floors

Review findings D1 (blocker) and 4.

- D1: buildChildElements assigned expanded children synthetic display rows as
  `host.track + index` — integers that can EQUAL a real clip's lane in another
  file (host on 0 with two children puts child #2 on 1). Lane grouping merges
  purely by track number, so the collision fused clips from different source
  files into one display lane, and lane-scoped actions (the gap menu) then
  batch-persisted a foreign file's clip. Children now take FRACTIONS strictly
  between the host's lane and the next integer — structurally unable to
  collide with any normalized lane, while still rendering as ordered rows
  under the host. Regression test pins the reviewer's exact two-file scenario.
- Finding 4: gap math compacted toward absolute 0, but an expanded child's
  display time is host-anchored — close/compact could drag it before its host
  window and persist a wrong (even negative) local time. All gap functions
  now take a lane FLOOR (laneGapFloor: 0 for ordinary lanes, the children's
  expandedParentStart for child lanes — single-origin per lane post-D1),
  threaded through the menu model, hover highlights, selected-lane strips,
  and both commits. Close-gap shifts clamp at the gap's own left edge.
…hbor identity

Review findings 1, 2, and 3.

- Finding 1: buildTrackInsertEdits normalized the FULL display set and
  persisted every shifted clip — writing host-lane numbers into OTHER
  composition files when expanded children were showing. The renumber write
  set is now the edited element's own source file (the sanctioned multi-write
  converges one FILE to lane space, never neighbors' files); foreign clips
  keep their authored tracks and re-derive display lanes. The locked-clip
  refusal scopes the same way. Expanded-origin elements refuse the insert
  outright (a new lane is a host-space renumber, meaningless in the child's
  file), and the mirrors restrict an expanded child's lane candidates to its
  own siblings' lanes — a sub-comp child still mirrors WITHIN its sub-comp
  (persisting the sibling's authored track) but can never land on a host lane
  with no same-file occupant. authoredTrackForLane's offset fallback rounds:
  fractional synthetic rows must never leak fractions into data-track-index.
- Finding 2: the mirror comparison sets required only sameSourceFile, but a
  file can contain several CSS stacking contexts and leaf z is only
  comparable within one. Both resolvers now scope by samePaintScope — same
  source file AND same stackingContextId (the file check also stops null root
  contexts of different files from comparing equal in the expanded view).
- Finding 3: the crossed-neighbor key was derived without selectorIndex, so
  duplicate class selectors (.sub) resolved to occurrence 0 — a different
  clip. The key now carries getSelectorIndex, matching how z-reorder entries
  derive theirs.
…on durable persists

Review findings 5 and 7.

- Finding 5: commitDomEditPatchBatches resolved successfully even when the
  server matched NO patch target — the z write never reached disk (the
  preview reloads to reconverge) yet the lane mirror still ran, desyncing
  track order from what actually paints. The commit now resolves a durability
  report ({allMatched, changed}; the save queue and commit types are generic
  over the result), and the mirror phase is skipped on allMatched === false.
- Finding 7: the z persist rides the DOM-edit save queue while the lane move
  rides the timeline/SDK path — two queues, so a second rapid gesture's z
  write could land BETWEEN the first gesture's z and lane phases. Every
  z-to-lane gesture (canvas z-order menu AND Layers-panel drag) now runs
  through runZLaneGesture: a single module-level tail that serializes the
  COMPLETE two-phase transaction, with unit tests for ordering, the
  durability gate, and queue resilience to failed gestures. The timeline
  lane-drag's inverse (move-then-z-sync) shares its phases' await ordering
  already; cross-gesture serialization for that path is noted as follow-up.
- LayersPanel's pure sort helpers moved to layersPanelSort.ts (600-line cap).
Review finding 6. finishGroupTimingGsapFallback mutates files sequentially
per clip; a late per-clip failure left the earlier rewrites on disk with no
aggregate history entry — unreachable by undo. foldGsapMutationIntoHistory
already snapshots every touched path before mutating; on a mutation failure
it now restores each path whose disk content changed (all-or-nothing batch),
reports restore errors without masking the original failure, and rethrows.
Regression test drives a two-clip batch whose second rewrite fails and
asserts the first clip's write is restored byte-identically.
@ukimsanov ukimsanov force-pushed the feat/track-paint-order-mirror branch from bf09849 to ba81c36 Compare July 14, 2026 10:08
@ukimsanov

Copy link
Copy Markdown
Collaborator Author

All nine findings from both reviews verified against the tree and fixed — every one was real. New head ba81c36 (rebased on origin/main). The push also carries this week's feature commits (gap-menu polish + hover/selection gap highlights, capcut zoom steps ×2 twice, the layers-drag→timeline mirror completing the three-way sync, flashless undo/redo, the left breathing pad before t=0); the review fixes are the five commits on top, one per finding group.

Per-finding:

  1. Insert renumbers crossed filesfix(studio): scope mirror references, insert writes... — the renumber write set is now the edited element's own source file only (the sanctioned multi-write converges one FILE to lane space); foreign clips keep their authored tracks and re-derive display lanes. Expanded-origin elements refuse inserts outright, and mirrors restrict an expanded child's candidates to its own siblings' lanes (it still mirrors within its sub-comp, persisting the sibling's authored track). authoredTrackForLane's offset fallback rounds so fractional synthetic rows can't leak into data-track-index.
  2. sameSourceFile ≠ stacking context → both resolvers now scope by samePaintScope = same file AND same stackingContextId (the file check also stops null root contexts of different files comparing equal). Tests for nested-context and cross-file-null cases.
  3. crossedKey missing selectorIndex → the key now carries getSelectorIndex, matching how z-reorder entries derive theirs; duplicate .sub selectors resolve to the right occurrence.
  4. Gap compaction from absolute 0 → all gap math takes a lane floor (laneGapFloor: 0 ordinary, the children's host-window start for expanded lanes), threaded through menu model, hover highlights, selection strips, and both commits. Tests.
  5. Unmatched z patch still mirroredcommitDomEditPatchBatches resolves a durability report ({allMatched, changed}); the lane mirror is skipped on allMatched === false (the preview reloads to reconverge). Applies to the canvas menu and the new Layers-drag path alike.
  6. Sequential GSAP batch, no rollbackfoldGsapMutationIntoHistory restores every touched path to its pre-batch snapshot when a later per-clip mutation fails (all-or-nothing); restore errors reported without masking the original. Regression test: two-clip batch, second rewrite fails, first write restored byte-identically.
  7. Two queues interleave → every z→lane gesture (canvas menu AND Layers drag) runs through runZLaneGesture, a single serialized tail for the complete two-phase transaction, unit-tested for ordering/gating/failure resilience. The lane-drag's inverse (move→z-sync) has an analogous pre-existing window — flagged for the parked follow-up PR alongside F6.

Deepwork:

  1. Synthetic track collision (blocker) → expanded children now take fractional display rows strictly between the host's lane and the next integer — structurally unable to merge with any normalized lane. Your exact two-file scenario is pinned as a regression test (useExpandedTimelineElements.test.ts).
  2. Cross-document z badge → resolved at the root: the z badge is removed entirely (product decision from Ular — the affordance never earned its place). No detector remains to mis-fire; the paint-order predicates are unchanged and now module-private.

Regression coverage added across the requested axes: mixed source files/stacking contexts, duplicate selectors, expanded-lane origins + floors, unmatched patches, late GSAP failure, and overlapping z gestures (serialization test). Full studio suite: 2,260 passing (the 18 known env-only failures unchanged).

@ukimsanov ukimsanov requested a review from miguel-heygen July 14, 2026 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants